Fix Clang warnings by using proper function prototypes in macros #179

ShravanDeva5327 · 2025-05-23T15:51:39Z

This PR updates the macros to specify (void) in argument lists for functions that take no arguments. This resolves warnings from
Clang(-Wstrict-prototypes).

Tested locally with Clang; no build issues observed.

Closes #178
Supersedes #115

fujitatomoya

@ShravanDeva5327 change looks good to me, but DCO is missing. can you address that?

Signed-off-by: Shravan Deva <[email protected]>

ShravanDeva5327 · 2025-05-23T15:58:56Z

@fujitatomoya I forgot to sign off the commit. I've added it now.

tracetools/include/tracetools/tracetools.h

christophebedard · 2025-05-23T16:37:44Z

By the way, it's good practice to reference any relevant issue or PR in the PR description. This way, reviewers can get the whole context behind a change. This also applies when creating issues, of course.

In this case, since this PR supersedes #115 (and since my comments on that PR may be relevant for this PR), it would be good to include "Supersedes #115" on a separate line in the PR description.

ShravanDeva5327 · 2025-05-23T17:19:32Z

By the way, it's good practice to reference any relevant issue or PR in the PR description. This way, reviewers can get the whole context behind a change. This also applies when creating issues, of course.

In this case, since this PR supersedes #115 (and since my comments on that PR may be relevant for this PR), it would be good to include "Supersedes #115" on a separate line in the PR description.

That makes sense, I’ve edited the PR description, and I’ll make sure to reference related issues or PRs in the future too

Signed-off-by: Shravan Deva <[email protected]>

christophebedard · 2025-05-23T18:18:48Z

Oh, and you can also include "Closes #178" in the PR description (again on a separate line). This way, the #178 issue will automatically get closed when this PR gets merged!

tracetools/include/tracetools/tracetools.h

Signed-off-by: Shravan Deva <[email protected]>

ShravanDeva5327 · 2025-05-28T14:04:12Z

This should hopefully resolve the warnings. Since I'm not able to reproduce them locally, I can't confirm if they're fully resolved.

@christophebedard Could you please trigger the CI job manually to check?

Signed-off-by: Shravan Deva <[email protected]>

christophebedard

I tried in an Ubuntu 24.04 Docker container and it seems like your new changes did the trick!

Here's a new job to validate that the warnings went away:

nightly_linux_clang_libcxx

If that looks good, I'll run full CI after you apply my suggestion.

tracetools/src/tracetools.c

tracetools/include/tracetools/tracetools.h

christophebedard · 2025-05-28T23:21:26Z

There seem to be weird issues with downloading from http://archive.ubuntu.com in the nightly_linux_clang_libcxx job. I retriggered it a second time.

christophebedard · 2025-05-28T23:22:50Z

..and I just remembered that I should use ci_linux_clang_libcxx instead of nightly_linux_clang_libcxx for manually-triggered jobs 😬

christophebedard · 2025-05-29T01:10:54Z

It failed once again due to an infrastructure issue. Here's a ci_linux_clang_libcxx job instead:

ci_linux_clang_libcxx

christophebedard · 2025-05-29T01:43:37Z

That failed again due to issues connecting to archive.ubuntu.com. Let's just wait a bit.

Signed-off-by: Shravan Deva <[email protected]>

ShravanDeva5327 · 2025-05-29T07:09:37Z

That failed again due to issues connecting to archive.ubuntu.com. Let's just wait a bit.

Looks like the same issue is causing GitHub actions to fail

  Err:1 http://archive.ubuntu.com/ubuntu noble-updates/main amd64 libapparmor1 amd64 4.0.1really4.0.1-0ubuntu0.24.04.4
    Connection failed [IP: 91.189.91.82 80]
  Err:2 http://archive.ubuntu.com/ubuntu noble/main amd64 sudo amd64 1.9.15p5-3ubuntu5
    Connection failed [IP: 185.125.190.83 80]
  E: Failed to fetch http://archive.ubuntu.com/ubuntu/pool/main/a/apparmor/libapparmor1_4.0.1really4.0.1-0ubuntu0.24.04.4_amd64.deb  Connection failed [IP: 91.189.91.82 80]
  E: Failed to fetch http://archive.ubuntu.com/ubuntu/pool/main/s/sudo/sudo_1.9.15p5-3ubuntu5_amd64.deb  Connection failed [IP: 185.125.190.83 80]
  E: Unable to fetch some archives, maybe run apt-get update or try with --fix-missing?

ahcorde · 2025-05-29T08:02:36Z

ci_linux_clang_libcxx

christophebedard

Looks good to me now, thanks for iterating!

Now it's just about validating that this fixes the clang warnings (which I think it should) and then running full CI.

christophebedard · 2025-05-29T17:37:28Z

The warnings indeed disappeared! https://ci.ros2.org/view/manual/job/ci_linux_clang_libcxx/85/clang/

Now I'll trigger full CI.

christophebedard · 2025-05-29T17:38:23Z

Pulls: #179
Gist: https://gist.githubusercontent.com/christophebedard/21ac843a08ababb31d6b90d0dfdad86f/raw/17e9b7e0005b9df94c92e8d33f4cb5849e71ec2f/ros2.repos
BUILD args: --packages-above-and-dependencies tracetools
TEST args: --packages-above tracetools
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/16119

Linux
Linux-aarch64
Linux-rhel
Windows

ahcorde

Thank you for fixing this!

ahcorde · 2025-05-30T08:47:19Z

https://github.com/Mergifyio backport kilted

mergify · 2025-05-30T08:47:28Z

backport kilted

✅ Backports have been created

#180 Fix Clang warnings by using proper function prototypes in macros (backport #179) has been created for branch kilted

Signed-off-by: Shravan Deva <[email protected]> (cherry picked from commit 2ca74e7)

christophebedard · 2025-05-30T14:52:11Z

Thank you for fixing this!

Yes, thank you @ShravanDeva5327!

… (#180) (cherry picked from commit 2ca74e7) Signed-off-by: Shravan Deva <[email protected]> Co-authored-by: Shravan Deva <[email protected]>

fujitatomoya reviewed May 23, 2025

View reviewed changes

Fix Clang warnings by using proper function prototypes in macros

519b532

Signed-off-by: Shravan Deva <[email protected]>

ShravanDeva5327 force-pushed the fix-clang-warnings branch from 75f3b4f to 519b532 Compare May 23, 2025 15:56

christophebedard self-requested a review May 23, 2025 16:02

christophebedard reviewed May 23, 2025

View reviewed changes

tracetools/include/tracetools/tracetools.h Outdated Show resolved Hide resolved

Fix macro expansion to avoid invalid function call

63c743a

Signed-off-by: Shravan Deva <[email protected]>

ShravanDeva5327 requested a review from christophebedard May 23, 2025 17:23

christophebedard mentioned this pull request May 23, 2025

Fixed warnings - strict-prototypes #115

Closed

christophebedard reviewed May 23, 2025

View reviewed changes

tracetools/include/tracetools/tracetools.h Outdated Show resolved Hide resolved

Fix more macro expansions to avoid invalid function calls

3ac816c

Signed-off-by: Shravan Deva <[email protected]>

ShravanDeva5327 requested a review from christophebedard May 24, 2025 13:46

Add _FUNC_* macros that expand to function names ros_trace_*

a100edd

Signed-off-by: Shravan Deva <[email protected]>

Replace ros_trace_enabled_* call with _FUNC_TRACEPOINT_ENABLED macro

32eedd0

Signed-off-by: Shravan Deva <[email protected]>

christophebedard reviewed May 28, 2025

View reviewed changes

tracetools/src/tracetools.c Show resolved Hide resolved

tracetools/include/tracetools/tracetools.h Outdated Show resolved Hide resolved

Fix consistency

3b12bf0

Signed-off-by: Shravan Deva <[email protected]>

christophebedard approved these changes May 29, 2025

View reviewed changes

ahcorde approved these changes May 30, 2025

View reviewed changes

ahcorde merged commit 2ca74e7 into ros2:rolling May 30, 2025
6 of 19 checks passed

mergify bot pushed a commit that referenced this pull request May 30, 2025

Fix Clang warnings by using proper function prototypes in macros (#179)

0ca8a19

Signed-off-by: Shravan Deva <[email protected]> (cherry picked from commit 2ca74e7)

mergify bot mentioned this pull request May 30, 2025

Fix Clang warnings by using proper function prototypes in macros (backport #179) #180

Merged

ShravanDeva5327 deleted the fix-clang-warnings branch May 30, 2025 09:22

Fix Clang warnings by using proper function prototypes in macros #179

Fix Clang warnings by using proper function prototypes in macros #179

Uh oh!

Conversation

ShravanDeva5327 commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fujitatomoya left a comment

Choose a reason for hiding this comment

Uh oh!

ShravanDeva5327 commented May 23, 2025

Uh oh!

Uh oh!

christophebedard commented May 23, 2025

Uh oh!

ShravanDeva5327 commented May 23, 2025

Uh oh!

christophebedard commented May 23, 2025

Uh oh!

Uh oh!

ShravanDeva5327 commented May 28, 2025

Uh oh!

christophebedard left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

christophebedard commented May 28, 2025

Uh oh!

christophebedard commented May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

christophebedard commented May 29, 2025

Uh oh!

christophebedard commented May 29, 2025

Uh oh!

ShravanDeva5327 commented May 29, 2025

Uh oh!

ahcorde commented May 29, 2025 • edited by christophebedard Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

christophebedard left a comment

Choose a reason for hiding this comment

Uh oh!

christophebedard commented May 29, 2025

Uh oh!

christophebedard commented May 29, 2025

Uh oh!

ahcorde left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ahcorde commented May 30, 2025

Uh oh!

mergify bot commented May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Backports have been created

Uh oh!

christophebedard commented May 30, 2025

Uh oh!

Uh oh!

ShravanDeva5327 commented May 23, 2025 •

edited

Loading

christophebedard left a comment •

edited

Loading

christophebedard commented May 28, 2025 •

edited

Loading

ahcorde commented May 29, 2025 •

edited by christophebedard

Loading

mergify bot commented May 30, 2025 •

edited

Loading